Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support rfc7523 private_key_jwt in client credentials flow #450

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SmotrovaLilit
Copy link

Implement JSON Web Token Profile for OAuth 2.0 Client Authentication in client credentials flow.

See https://tools.ietf.org/html/rfc7523
See https://openid.net/specs/openid-connect-core-1_0.html

Fixes #433

@google-cla google-cla bot added the cla: yes label Oct 25, 2020
@SmotrovaLilit SmotrovaLilit force-pushed the issue-433-add-support-rfc7523-client-credentials branch 2 times, most recently from 06ee726 to 56f9a0b Compare October 25, 2020 22:06
@gopherbot
Copy link
Contributor

This PR (HEAD: 56f9a0b) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/oauth2/+/264959 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 1:

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
A maintainer will review your change and provide feedback. See
https://golang.org/doc/contribute.html#review for more info and tips to get your
patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.


Please don’t reply on this GitHub thread. Visit golang.org/cl/264959.
After addressing review feedback, remember to publish your drafts!

@SmotrovaLilit SmotrovaLilit force-pushed the issue-433-add-support-rfc7523-client-credentials branch from 56f9a0b to d4025d6 Compare October 25, 2020 22:22
@gopherbot
Copy link
Contributor

This PR (HEAD: d4025d6) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/oauth2/+/264959 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gbolo
Copy link

gbolo commented Jan 18, 2021

@SmotrovaLilit thanks for adding this much needed feature. Any progress on getting this merged? I don't notice any code review on the Gerrit link yet...

@SmotrovaLilit
Copy link
Author

Hello. Nobody has started the code review yet. Could I do something to push forward the code review? Either should I wait for some time?

@pawelaugustyn
Copy link

If there's anything I can do to push forward the code review process I'll do that immediately. This is something that I was looking for! @SmotrovaLilit thanks for taking an initiative to proceed with adding this feature.

@dudududi
Copy link

Hello @adg @rakyll @bradfitz ,
I am mentioning you as I can see you as one of the most active contributors. Could you advice us what we can do to have it reviewed and merged? We are really looking forward for this feature!

@adg
Copy link
Contributor

adg commented Apr 21, 2021

AFAIK there are no active maintainers of this repository at the moment. Maybe @Sajmani knows?

@diefans
Copy link

diefans commented May 18, 2021

@dudududi maybe just the wrong people... @codyoss @ScruffyProdigy can you help to get this merged/reviewed?

@gopherbot
Copy link
Contributor

Message from Kamil Dudek:

Patch Set 2: Code-Review+1

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/264959.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Heschi Kreinick:

Patch Set 2:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/264959.
After addressing review feedback, remember to publish your drafts!

@sheen4n
Copy link

sheen4n commented May 28, 2021

Hi ! Seems like the code is ready, just need a code review or merge from @codyoss @ScruffyProdigy ? Thanks.

@codyoss
Copy link
Member

codyoss commented Jun 1, 2021

I will try to take a look at this later this week. I need to go over the RFC.

@sheen4n
Copy link

sheen4n commented Jun 25, 2021

I will try to take a look at this later this week. I need to go over the RFC.

Hi cody, may I check how was it on your end? thank you very much.

@CVMarkBradley
Copy link

Any movement on this yet?

@diefans
Copy link

diefans commented Jul 23, 2021

@codyoss Is there anything somebody can do to accelerate the review process?

@sheen4n
Copy link

sheen4n commented Aug 3, 2021

Seems like the code is ready, @codyoss do you mind reviewing? thank you so much.

@diefans
Copy link

diefans commented Sep 10, 2021

The impression is sneaking into my mind, that either

  • google is not interested in opensource or
  • people at google have negative incentives in spending some extra time outside their regular work or
  • this PR is crap, but nobody is able to communicate the flaws

@gopherbot
Copy link
Contributor

Message from Kyle Lemons:

Patch Set 2:

(21 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/264959.
After addressing review feedback, remember to publish your drafts!

@diefans
Copy link

diefans commented Sep 10, 2021

first: thanks to Kyle Lemons
second: to all you thumb down people: this was my personal impression I honestly reflected and obviously the third point was somehow true - so what's your problem? People were asking over and over in a polite way how they can help and earned ignorance (from all of you) - now that I came up with an insight to the real point - you don't like it?

@codyoss
Copy link
Member

codyoss commented Sep 10, 2021

Hey all, I would still like to help take a look at this sometime in the near future(most likely start Oct). It had fallen off my radar and other priorities have needed to take precedence. I generally work just in within the google subdir for this repo but would like to try to help get these changes moving. I still need to make some time to look over the RFC. Thank you for your patience.

@greatvovan
Copy link

@diefans

  1. These thumbs down are likely from people from Reddit there, who learned that you was "very rude" and decided to punish you (without proper reading) because blind downvoting is something Reddit people are good at.
  2. Your wording is indeed prone to misunderstanding. I see you are assuming that this PR is crap, but people read only "this PR is crap" and lose the context. Though even assuming that PR is bad is not polite in this wording.

@gopherbot
Copy link
Contributor

Message from Kyle Lemons:

Patch Set 2:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/264959.
After addressing review feedback, remember to publish your drafts!

@diefans
Copy link

diefans commented Sep 11, 2021

@greatvovan ... it might be, that I was overemphasizing (e.g. "crap") and I apologize - but nevertheless, it brought some kind of progression... ;)

@gopherbot
Copy link
Contributor

Message from Dmitriy Smotrov:

Patch Set 2:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/264959.
After addressing review feedback, remember to publish your drafts!

@Asday
Copy link

Asday commented Sep 12, 2021

@diefans

  1. These thumbs down are likely from people from Reddit there, who learned that you was "very rude" and decided to punish you (without proper reading) because blind downvoting is something Reddit people are good at.
  2. Your wording is indeed prone to misunderstanding. I see you are assuming that this PR is crap, but people read only "this PR is crap" and lose the context. Though even assuming that PR is bad is not polite in this wording.

Please do not assume my intentions. You have no idea why I thumbs down'd the comment, and you are not in a position to speculate. You also have no evidence that I haven't properly read a comment, nor that I have managed to forget its context in the time it took me to reach for the button.

Perhaps I can correct your misunderstanding.

google is not interested in opensource or

This is a strawman - google is one of if not the biggest corporate contributors to open source in the world. The way this reads is "oh yeah, leap that wall if you're so strong, bet you can't".

people at google have negative incentives in spending some extra time outside their regular work or

This is also a strawman. @diefans has no apparent knowledge of the inner workings of google, and again, this reads as being intentionally incorrect so as to rouse a response.

this PR is crap, but nobody is able to communicate the flaws

And yet more along the same lines, "or maybe the PR is crap" is posed in a seemingly ironic manner, followed by yet another strawman - google is not google for no reason. If there are only a few companies in the world with an employee that can communicate the flaws in a PR, one of those companies will be google. It seems like it's just trying to get a rise.

All in all, @diefans' comment is incredibly childish, and serves absolutely no purpose on the PR. It doesn't further discussion in any way, either by @'ing relevant people who haven't seen an update, nor by "communicating flaws". Its tone in fact brings the rest of the PR down by association. It's an embarrassment. It is one of the worst comments I've seen on a PR in recent memory, and I read a lot of PRs in dumb projects. If I had the privelege, I would have deleted it. That's why I thumbs down'd it.

@diefans
Copy link

diefans commented Sep 12, 2021

@Asday (and this is my last comment, sorry) after 11 months of desperate begging for something, I decided (also in face of my actual project deliverables, constraints and pressures, etc.) to stop that pray.

Fact is: this PR (and all its advocates) were starved to death since the beginning. I would be surprised, if @SmotrovaLilit is still invested.

Here is a review of your post, which smells (after more than thirty years working with logic) quite inconsistent...

and serves absolutely no purpose on the PR

The purpose was to move it in an inconvenient way

It doesn't further discussion in any way, either by @'ing relevant people who haven't seen an update

I was looking up people with recent commits and involved them

nor by "communicating flaws"

hmmm.. I was asking to provide further information

You also have no evidence that I haven't properly read a comment

which is proven now by yourself

@diefans has no apparent knowledge of the inner workings of google

...a friend was telling me something about this (why would such crazy things come up in my mind?)

google is one of if not the biggest corporate contributors to open source in the world

I know this, that's why these 11 months of "silence" are so ridiculous and unaccountable to me

If there are only a few companies in the world with an employee that can communicate the flaws in a PR, one of those companies will be google.

But it hasn't ... what "unable" means

Its tone

the only "tone" in my comment rests in the word "crap", where everybody, connecting the dots (since I am begging for that "crap" and would buy it even as it is) must see that I am exaggerating.

Isn't your tone worse - I mean you throwing around the same stuff, you accuse somebody else of:

"You have no idea ... ", "you are not in a position ...", "I can correct your ...", "has no apparent knowledge...", "incredibly", "childish", "absolutely", "worst", "dumb"

I think you should apply your own rule to yourself, namely:

If I had the privelege, I would have deleted it.

So I close this "Pandora's box" by quoting your own site:

Right to be Offensive
It follows that every individual has an obligation to be responsible for their own feelings. Expecting everyone around you to coddle you and protect your feelings is an expectation you should have grown out of before leaving school.

@codyoss
Copy link
Member

codyoss commented Sep 12, 2021

May I please remind everyone here to be respectful make sure discussion is in line with our code of conduct Code of Conduct. Let's also try to keep discussion on topic of the PR itself. Thanks.

@SmotrovaLilit SmotrovaLilit changed the title Add support rfc7523 in client credentials flow Add support rfc7523 private_key_jwt in client credentials flow Oct 20, 2021
@SmotrovaLilit SmotrovaLilit force-pushed the issue-433-add-support-rfc7523-client-credentials branch from d4025d6 to 267a86f Compare October 20, 2021 17:11
@gopherbot
Copy link
Contributor

This PR (HEAD: 267a86f) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/oauth2/+/264959 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Lilit Smotrova:

Patch Set 3:

(18 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/264959.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Lilit Smotrova:

Patch Set 4:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/264959.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Kamil Dudek:

Patch Set 2: Code-Review+1

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/264959.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Heschi Kreinick:

Patch Set 2:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/264959.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Kyle Lemons:

Patch Set 2:

(21 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/264959.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Kyle Lemons:

Patch Set 2:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/264959.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Dmitriy Smotrov:

Patch Set 2:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/264959.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Lilit Smotrova:

Patch Set 3:

(18 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/264959.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Lilit Smotrova:

Patch Set 4:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/264959.
After addressing review feedback, remember to publish your drafts!

@ahrberg
Copy link

ahrberg commented Mar 8, 2022

This is a very much appreciated feature. Is this close to get merged? @codyoss?

@MichaelHindley
Copy link

This feature is highly anticipated for our identity management solutions, is there something that can be done to help move it along?

@dsxack
Copy link

dsxack commented Aug 4, 2022

Looks like this project is dead, but not – new PRs continue merge into it.

Maybe @rsc could help with it? He is the author of latest commits in project and I don't know anyone else, who can help with it.

@SmotrovaLilit SmotrovaLilit force-pushed the issue-433-add-support-rfc7523-client-credentials branch from 267a86f to 1971a7d Compare November 18, 2023 11:37
@gopherbot
Copy link
Contributor

Message from Dmitriy Smotrov:

Patch Set 4:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/264959.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: 1971a7d) has been imported to Gerrit for code review.

Please visit Gerrit at https://go-review.googlesource.com/c/oauth2/+/264959.

Important tips:

  • Don't comment on this PR. All discussion takes place in Gerrit.
  • You need a Gmail or other Google account to log in to Gerrit.
  • To change your code in response to feedback:
    • Push a new commit to the branch used by your GitHub PR.
    • A new "patch set" will then appear in Gerrit.
    • Respond to each comment by marking as Done in Gerrit if implemented as suggested. You can alternatively write a reply.
    • Critical: you must click the blue Reply button near the top to publish your Gerrit responses.
    • Multiple commits in the PR will be squashed by GerritBot.
  • The title and description of the GitHub PR are used to construct the final commit message.
    • Edit these as needed via the GitHub web interface (not via Gerrit or git).
    • You should word wrap the PR description at ~76 characters unless you need longer lines (e.g., for tables or URLs).
  • See the Sending a change via GitHub and Reviews sections of the Contribution Guide as well as the FAQ for details.

Implement JSON Web Token Profile for OAuth 2.0 Client Authentication in client credentials flow.

See https://tools.ietf.org/html/rfc7523
See https://openid.net/specs/openid-connect-core-1_0.html

Fixes golang#433
@SmotrovaLilit SmotrovaLilit force-pushed the issue-433-add-support-rfc7523-client-credentials branch from 1971a7d to 7b46ef7 Compare November 18, 2023 11:54
@gopherbot
Copy link
Contributor

This PR (HEAD: 7b46ef7) has been imported to Gerrit for code review.

Please visit Gerrit at https://go-review.googlesource.com/c/oauth2/+/264959.

Important tips:

  • Don't comment on this PR. All discussion takes place in Gerrit.
  • You need a Gmail or other Google account to log in to Gerrit.
  • To change your code in response to feedback:
    • Push a new commit to the branch used by your GitHub PR.
    • A new "patch set" will then appear in Gerrit.
    • Respond to each comment by marking as Done in Gerrit if implemented as suggested. You can alternatively write a reply.
    • Critical: you must click the blue Reply button near the top to publish your Gerrit responses.
    • Multiple commits in the PR will be squashed by GerritBot.
  • The title and description of the GitHub PR are used to construct the final commit message.
    • Edit these as needed via the GitHub web interface (not via Gerrit or git).
    • You should word wrap the PR description at ~76 characters unless you need longer lines (e.g., for tables or URLs).
  • See the Sending a change via GitHub and Reviews sections of the Contribution Guide as well as the FAQ for details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support rfc 7523